Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Week7] 7주차 필수과제 #16

Open
wants to merge 40 commits into
base: develop
Choose a base branch
from
Open

[Week7] 7주차 필수과제 #16

wants to merge 40 commits into from

Conversation

hyeeum
Copy link
Contributor

@hyeeum hyeeum commented Dec 17, 2024

Related issue 🛠

Work Description ✏️

  • MVI 설계
  • MVI 적용

Screenshot 📸

  • 이전화면과 동일합니다 :)

Uncompleted Tasks 😅

  • 유저의 정보(User)를 datastore 이용하기
  • Log -> Timber
  • 6주차 코드리뷰 답글달기!

To Reviewers 📢

아직 6주차 PR을 안닫아서 끌어당겨 와서 PR을 올렸습니다 :)
생각해보니...그냥 병합했으면 되는 PR이네요.. 15번 이슈번호가 있는 PR부터 봐주시면 될 것 같습니다
급하게 하느라 아직 제대로 MVI를 반영하진 않은 것 같습니다.. 시험 목요일에 끝나서 얼른얼른.. 코드리뷰 마감전까지는 수정할 수 있도록 하겠습니다..!! 죄송합니다 ㅠㅠ

@hyeeum hyeeum self-assigned this Dec 17, 2024
@HAJIEUN02
Copy link

HAJIEUN02 commented Dec 18, 2024

시험 끝나고 코드 더 변경하시면 그때 한번에 코리할게요!_!
시험 화이팅 혜음,, 젤 늦게 끝나는 혜음이ㅜ

Copy link
Contributor

@1971123-seongmin 1971123-seongmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제 하시느라 고생하셨어요. 시험 힘내세요 🥲🥲

Comment on lines +20 to +22
abstract fun bindsSignInRepository(
signInRepositoryImpl: SignInRepositoryImpl
): SignInRepository

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repository랑 RepositoryImpl을 구분하는 이유는 무엇인가요? 어차피 구현체 하나라서 인터페이스 없이 Repository를 구체 클래스로 제공해도 무방할 것 같아서요

import org.sopt.and.domain.repository.MyRepository
import javax.inject.Inject

class MyUseCase @Inject constructor(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 객체는 아래처럼 쓰일 텐데요.

val myUseCase = MyUseCase(...)
val ?? = myUseCase()

무엇을 하는 UseCase인지 알기 어려워 보입니다.

GetHobbyUseCase라고 하심은 어떠신지요?


더 나아가서 이 UseCase는 단순히 하나의 Repository 함수 호출을 감싸는 수준에 불과합니다.
MyUseCase 객체를 관리해서 얻는 이점이 뭐가 있나요? 그냥 ViewModel에서 Repository 객체를 받아서 사용하면 되는 것 아닌가요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UseCase들의 이름과 필요성에 대해서 고민이 필요합니다. 다른 UseCase들도 다요.

Copy link
Contributor

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다

fun getUserHobby() {
viewModelScope.launch {
wavveRepository.getHobby().onSuccess { hobbyEntity ->
getHobbyUseCase.invoke().onSuccess { hobbyEntity ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
getHobbyUseCase.invoke().onSuccess { hobbyEntity ->
getHobbyUseCase().onSuccess { hobbyEntity ->

이렇게도 사용할 수 있을 것 같아요.
invoke 함수가 무엇인지 공부해 봅시다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] 7주차 필수 과제
5 participants